-
-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(cockroachdb): to use request driven options #2883
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
fbfb05c
to
6dc9c5c
Compare
This should be split out into components if we want to go down this route. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did an initial pass and the overall feedback is positive. I detected the following possible chunks of separate PRs, please tell me what you think:
- wait.walk for traversing multi strategies
- wait for TLS
- cockroachdb refactor
I think it would be much clearer for the review, but also for the release notes, as consumers would discover the new wait strategies as part of it.
That's pretty much my thought's too, which is why I've left in draft. |
158644d
to
83a7838
Compare
df5766a
to
81290cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Just a few comments, minor ones.
@mdelapenya I've added clarification to the Run method about what version is supported. I tested all the way back to v22.2.0, which was the first version which introduced the init_success file creation once the cluster init has completed. |
…list of statements
Co-authored-by: Steven Hartland <[email protected]>
Co-authored-by: Steven Hartland <[email protected]>
… in defaultOptions
Co-authored-by: Manuel de la Peña <[email protected]>
… to DefaultStatements
Simplify the connection handling in cockroachdb so that ConnectionString can be used without the user doing extra work to handle TLS if enabled. Deprecate TLSConfig which is no longer needed separately. BREAKING_CHANGE: This now returns a registered connection string so is no longer compatible with pgx.ParseConfig, use ConnectionConfig for this use case instead.
Refactor cockroachdb module to to use request driven options, simplifying the flow.
Various fixes and remove the ability to configure certs using the default generated ones instead.
Add the missing data file and remove unused constants.
Extract TLS certificate wait strategy into a dedicated wait type so it can be reused. Implement walk method which can be used to identify wait strategies in a request chain. Use embed to simplify wait test loading of certs.
Remove the now unused customizer.
Fix lint style issue for blank line.
Clarified the impact of run defaults.
0a185ed
to
51f572d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
* main: fix: container binds syntax (testcontainers#2899) refactor(cockroachdb): to use request driven options (testcontainers#2883) chore(deps): bump actions/setup-go from 5.0.0 to 5.1.0 (testcontainers#2904) chore(deps): bump ossf/scorecard-action from 2.3.1 to 2.4.0 (testcontainers#2903) chore(deps): bump test-summary/action from 2.3 to 2.4 (testcontainers#2902) feat(wait): strategy walk (testcontainers#2895) feat(wait): tls strategy (testcontainers#2896)
* main: (234 commits) chore(ci): add Github labels based on PR title (testcontainers#2914) chore(gha): Use official setup-docker-action (testcontainers#2913) chore(ci): enforce conventional commits syntax in PR titles (testcontainers#2911) feat(nats): WithConfigFile - pass a configuration file to nats server (testcontainers#2905) chore: enable implicit default logger only in testing with -v (testcontainers#2877) fix: container binds syntax (testcontainers#2899) refactor(cockroachdb): to use request driven options (testcontainers#2883) chore(deps): bump actions/setup-go from 5.0.0 to 5.1.0 (testcontainers#2904) chore(deps): bump ossf/scorecard-action from 2.3.1 to 2.4.0 (testcontainers#2903) chore(deps): bump test-summary/action from 2.3 to 2.4 (testcontainers#2902) feat(wait): strategy walk (testcontainers#2895) feat(wait): tls strategy (testcontainers#2896) docs: better contribution guidelines (testcontainers#2893) fix(influxdb): Respect custom waitStrategy (testcontainers#2845) fix: only upload to sonar on ubuntu-latest (testcontainers#2891) fix: build artifact name properly (testcontainers#2890) fix: do not run sonar upload when ryuk is disabled (testcontainers#2889) fix: update GH actions for uploading/downloading artifacts (testcontainers#2888) feat(ci): Enable master moby with rootless (testcontainers#2880) fix(redpanda): temporary file use ...
* main: feat(gcloud)!: add support to seed data when using RunBigQueryContainer (testcontainers#2523) security(deps): bump golang.org/x/crypto from 0.28.0 to 0.31.0 (testcontainers#2916) chore(ci): add Github labels based on PR title (testcontainers#2914) chore(gha): Use official setup-docker-action (testcontainers#2913) chore(ci): enforce conventional commits syntax in PR titles (testcontainers#2911) feat(nats): WithConfigFile - pass a configuration file to nats server (testcontainers#2905) chore: enable implicit default logger only in testing with -v (testcontainers#2877) fix: container binds syntax (testcontainers#2899) refactor(cockroachdb): to use request driven options (testcontainers#2883)
Refactor cockroachdb module to to use request driven options simplifying the flow.
Clean up tests eliminating the use of suite which significantly speeds up test runs.